Feature/agent isolation chat#26
Conversation
核心改进: - 意图分类网关(intent_gateway): 将用户请求分类为GENERAL_CHAT/TOOL_CALL/LOCAL_TOOL - 工具懒加载(tool_lazy_loader): 仅TOOL_CALL类型才注入匹配场景的工具 - 本地工具处理(local_handler): 时间/天气等本地工具直接处理,不经过LLM - 工具结果处理器(tool_result_processor): 统一处理工具执行结果 新增模块: - app/mcp/: MCP协议服务器实现(time_server, weather_server) - app/utils/: 工具相关工具模块 修改文件: - chat.py: 集成意图识别和工具按需注入 - registry.py: 扩展技能注册表 - adapter.py/providers.py: LLM适配器优化
| @@ -0,0 +1,82 @@ | |||
| import { WebContentsView, BrowserWindow } from 'electron' | |||
| @@ -0,0 +1,82 @@ | |||
| import { WebContentsView, BrowserWindow } from 'electron' | |||
| import { createBrowserView, attachView, detachView, setViewBounds, isViewDestroyed } from './view' | |||
| @@ -0,0 +1,82 @@ | |||
| import { WebContentsView, BrowserWindow } from 'electron' | |||
| import { createBrowserView, attachView, detachView, setViewBounds, isViewDestroyed } from './view' | |||
| import { calculateBounds } from './view' | |||
| import { WebContentsView, BrowserWindow } from 'electron' | ||
| import { createBrowserView, attachView, detachView, setViewBounds, isViewDestroyed } from './view' | ||
| import { calculateBounds } from './view' | ||
| import { DEFAULT_BROWSER_CONFIG } from './types' |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Walkthrough新增文件上传解析、三级意图网关、工具调用循环(流/非流)、本地 time/weather 工具、工具执行/处理管道、LLM 提供者返回形态变更、MCP 服务与前端文件/记忆/推理渲染,移除旧流水线与事件总线。 Changes主功能合并(意图、工具、文件)
工具相关框架
本地工具与技能注册
LLM Provider 与适配器变更
MCP 服务与测试
前端改动(文件、搜索、记忆、推理显示)
架构清理
Sequence Diagram(s)sequenceDiagram
participant User
participant FE as Frontend
participant FileAPI as /upload/forward
participant ChatAPI as /chat
participant Intent as IntentGateway
participant LLMAd as LLMAdapter
participant ToolExec as ToolExecutor
participant Local as LocalTools
User->>FE: 发送消息(可含文件)
FE->>FileAPI: POST /upload/forward(若上传)
FileAPI-->>FE: {type, content, filename}
FE->>ChatAPI: POST /chat/completions (message + file_content + search_results)
ChatAPI->>Intent: classify_request()
Intent-->>ChatAPI: LOCAL_TOOL / TOOL_CALL / GENERAL_CHAT
alt LOCAL_TOOL
ChatAPI->>Local: handle_local_tool_request()
Local-->>ChatAPI: 快速回复
else TOOL_CALL
ChatAPI->>LLMAd: chat(messages, tools, return_raw=True)
LLMAd->>ToolExec: 返回 tool_calls -> 执行 execute_tool_chain()
ToolExec-->>ChatAPI: tool results
ChatAPI->>LLMAd: chat(messages + tool results)
LLMAd-->>ChatAPI: final content / reasoning
else GENERAL_CHAT
ChatAPI->>LLMAd: chat(messages)
LLMAd-->>ChatAPI: content/reasoning
end
ChatAPI-->>FE: SSE / HTTP 响应(含 reasoning_content)
FE->>User: 渲染回复与可折叠推理
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
| current_date_info: str | None = None | ||
| if "get_current_time" in tool_names and "web_search" in tool_names: | ||
| try: | ||
| time_args = _extractor.extract("get_current_time", user_query) |
| 6: "星期日", | ||
| } | ||
|
|
||
| _WEEKDAY_NAMES_SHORT = { |
| # ---- 多轮对话阈值(秒)---- | ||
| _REPEAT_SAME_MINUTE = 60 # 1分钟内重复查询 → "还是XX时间哦" | ||
| _REPEAT_NEAR_MINUTE = 120 # 2分钟内重复查询 → "距离上次才过了X分钟" | ||
| _REPEAT_MAX_WINDOW = 180 # 超过3分钟视为正常查询(大于 NEAR 阈值) |
| or timezone != _time_tool_timezone | ||
| or agent_id != _time_tool_agent_id): | ||
| _time_tool_instance = TimeTool(timezone=timezone, agent_id=agent_id) | ||
| _time_tool_timezone = timezone |
| or agent_id != _time_tool_agent_id): | ||
| _time_tool_instance = TimeTool(timezone=timezone, agent_id=agent_id) | ||
| _time_tool_timezone = timezone | ||
| _time_tool_agent_id = agent_id |
| import time | ||
| import asyncio | ||
| from datetime import datetime, timezone | ||
| from collections.abc import AsyncIterator |
| from app.core.config import settings | ||
| from app.utils.intent_gateway import classify_request, RequestType | ||
| from app.utils.tool_lazy_loader import get_matched_tools | ||
| from app.utils.tool_result_processor import process_tool_result |
| from app.utils.tool_lazy_loader import get_matched_tools | ||
| from app.utils.tool_result_processor import process_tool_result | ||
| from app.utils.local_handler import handle_local_tool_request | ||
| from app.utils.tool_executor import execute_tool_chain, build_tool_summary, execute_single_tool |
| python -m app.mcp.tests.test_mcp_protocol | ||
| """ | ||
|
|
||
| import sys |
| """ | ||
|
|
||
| import sys | ||
| import os |
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/renderer/src/views/WorkspaceView.vue (1)
263-266:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSanitize Markdown output to prevent XSS injection via
v-html.
marked.parse()returns unsanitized HTML. In marked 18.0.1 (your current version), there is no built-in sanitization—the official documentation explicitly warns: "Marked does not sanitize the output HTML." The rendered output is injected directly into the DOM viav-html="renderMarkdown(msg.content)"at line 695, allowing any HTML/JavaScript in assistant or tool-echoed content (e.g.,<img src=x onerror=alert()>,<script>, orjavascript:URIs in links) to execute in the Electron renderer process.DOMPurify is already in your dependencies (
^3.4.2). Use it to sanitize before injection:Suggested fix
import { marked } from 'marked' +import DOMPurify from 'dompurify' const renderMarkdown = (text: string): string => { if (!text) return '' - return marked.parse(text) as string + return DOMPurify.sanitize(marked.parse(text) as string) }Also applies to lines 694–699 where message content is rendered.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/views/WorkspaceView.vue` around lines 263 - 266, The renderMarkdown function returns raw HTML from marked.parse which is injected with v-html (e.g., in the message rendering around renderMarkdown(msg.content)), enabling XSS; fix it by importing DOMPurify and returning DOMPurify.sanitize(marked.parse(text)) from renderMarkdown (i.e., sanitize the marked output before returning), so all uses of renderMarkdown (including the v-html bindings) receive sanitized HTML; ensure the import and call reference the renderMarkdown function and sanitize every place renderMarkdown(msg.content) is used.
♻️ Duplicate comments (1)
frontend/src/main/services/browser/search.ts (1)
1-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused imports.
Multiple unused imports have been identified:
WebContentsView,attachView,detachView,setViewBounds,calculateBounds, andDEFAULT_BROWSER_CONFIG.🧹 Proposed cleanup
-import { WebContentsView, BrowserWindow } from 'electron' -import { createBrowserView, attachView, detachView, setViewBounds, isViewDestroyed } from './view' -import { calculateBounds } from './view' -import { DEFAULT_BROWSER_CONFIG } from './types' +import { BrowserWindow } from 'electron' +import { createBrowserView, isViewDestroyed } from './view'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/main/services/browser/search.ts` around lines 1 - 4, The listed imports are unused—remove the unused specifiers WebContentsView, attachView, detachView, setViewBounds, calculateBounds, and DEFAULT_BROWSER_CONFIG from the top of frontend/src/main/services/browser/search.ts so only the actually used symbols (e.g. createBrowserView, BrowserWindow or any others referenced later in the file) remain; update the import statements from './view' and './types' to only export the symbols that are referenced by functions like createBrowserView and any BrowserWindow usage to eliminate linter/compile warnings.
🟡 Minor comments (11)
backend/app/utils/tool_parameter_extractor.py-184-185 (1)
184-185:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConsider the UX impact of appending "时间" to short queries.
When a query is 3 characters or fewer, the code unconditionally appends " 时间". This might produce awkward search queries for non-time-related searches (e.g., "红楼梦" → "红楼梦 时间").
Consider restricting this suffix to queries that match time-related patterns or removing it if the search intent is not time-based.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/tool_parameter_extractor.py` around lines 184 - 185, The code currently appends the suffix " 时间" to any short query (variable query) when len(query) <= 3; change this to only append when the query appears time-related by checking for time patterns (e.g., digits or year tokens like "年", "月", "日", "年代", or explicit time words) using a simple regex, or replace the unconditional append with a call to an intent check (e.g., an is_time_query(query) helper) that returns true only for time-related queries; update the block that references query in tool_parameter_extractor.py to perform the pattern/intent check before adding " 时间" so non-time short queries (e.g., "红楼梦") are left unchanged.frontend/src/renderer/src/components/FilePreview.vue-19-45 (1)
19-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd error handling for base64 decoding.
The
handleDownloadfunction assumesfileContentis a well-formed data URL for images. If the format is unexpected (e.g., missing comma, invalid base64),split()oratob()will throw, leaving the user with no feedback.Wrap the decoding logic in a try-catch block and show a user-friendly error message on failure.
🛡️ Proposed error handling
const handleDownload = () => { if (!props.fileContent) return + try { let blob: Blob if (isImage.value) { const base64Data = props.fileContent.split(',')[1] + if (!base64Data) throw new Error('Invalid image data') const byteString = atob(base64Data) const mimeType = props.fileContent.split(':')[1].split(';')[0] const ab = new ArrayBuffer(byteString.length) const ia = new Uint8Array(ab) for (let i = 0; i < byteString.length; i++) { ia[i] = byteString.charCodeAt(i) } blob = new Blob([ab], { type: mimeType }) } else { blob = new Blob([props.fileContent], { type: 'text/plain' }) } const url = URL.createObjectURL(blob) const a = document.createElement('a') a.href = url a.download = props.fileName document.body.appendChild(a) a.click() document.body.removeChild(a) URL.revokeObjectURL(url) + } catch (err) { + console.error('Download failed:', err) + alert('下载失败,请稍后重试') + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/components/FilePreview.vue` around lines 19 - 45, The handleDownload function assumes props.fileContent is a valid image data URL and decodes it without protection; wrap the image decoding block (the code that reads base64Data, calls atob, computes mimeType, fills the ArrayBuffer and creates the Blob) in a try-catch that catches errors from split/atob/decoding, aborts the download flow on failure, and surfaces a user-friendly error (e.g., via an alert, a toast, or emitting an error event) so the user gets feedback; ensure you do not create the object URL or trigger a click if decoding failed and keep references to isImage and props.fileContent/fileName to locate the code.backend/app/utils/intent_gateway.py-317-323 (1)
317-323:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove duplicate "油价" entry.
Line 321 contains a duplicate
"油价"entry in the_TOOL_KEYWORDS_REALTIMEset. While Python will automatically deduplicate this, it should be cleaned up for code clarity.🧹 Proposed fix
_TOOL_KEYWORDS_REALTIME = { "股价", "股票", "行情", "汇率", "油价", "金价", "房价", "限行", "限号", "停水", "停电", "快递", "物流", "招聘", "求职", "签证", "出入境", "入境政策", - "油价", "汽油价", "黄金价", "二手房", "均价", + "汽油价", "黄金价", "二手房", "均价", }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/intent_gateway.py` around lines 317 - 323, Remove the duplicate "油价" entry from the _TOOL_KEYWORDS_REALTIME set to improve clarity; locate the _TOOL_KEYWORDS_REALTIME definition in intent_gateway.py and delete one of the two "油价" items so each keyword appears only once.backend/app/utils/intent_gateway.py-137-148 (1)
137-148:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove duplicate "发布会" entry.
Line 147 contains a duplicate
"发布会"entry in theevent_date_keywordsset. While Python will automatically deduplicate this, it indicates a copy-paste error and should be cleaned up for code clarity.🧹 Proposed fix
self.event_date_keywords: set[str] = { "软考", "考研", "高考", "中考", "国考", "省考", "考公", "公务员", "事业编", "选调", "教资", "法考", "注会", "一建", "二建", "复试", "笔试", "面试", "报名", "准考证", "成绩", "录取", "分数线", "世界杯", "奥运会", "亚运会", "世博会", "欧冠", "NBA", "欧洲杯", "亚洲杯", "全运会", "上映", "开售", "预售", "发售", "发布", "开学", "放假", "开学季", "毕业", "春运", "假期", "调休", - "发布会", "发布会", "直播", + "发布会", "直播", }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/intent_gateway.py` around lines 137 - 148, The set event_date_keywords in intent_gateway.py contains a duplicate "发布会"; remove the redundant "发布会" entry from the self.event_date_keywords initialization so the set literal is clean (keep only one "发布会" in the set defined in the __init__ where event_date_keywords is declared).frontend/src/renderer/src/components/FileUpload.vue-24-36 (1)
24-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFileCard :size="0" should reflect actual file size.
The
FileCardcomponent receives:size="0"which doesn't represent the actual uploaded file size. This will display incorrect information to users.Root cause: The
useFileUploadcomposable doesn't store the originalFileobject's size. WhenuploadAndForward(file)is called, only the file name is stored inuploadingFile.value.name, butfile.sizeis lost.Suggested fix:
- In
useFileUpload.ts, addfileSizeto the reactive state- Store
file.sizewhen upload starts- Pass it to FileCard as
:size="fileSize"🔧 Proposed fix
In
useFileUpload.ts:const uploadingFile = ref<{ name: string; status: 'uploading' | 'success' | 'failed'; type?: string; result?: string; error?: string } | null>(null) const isUploading = ref(false) const parsedContent = ref('') const fileType = ref('') const fileName = ref('') +const fileSize = ref(0) export function useFileUpload() { const uploadAndForward = async (file: File): Promise<string> => { ... uploadingFile.value = { name: file.name, status: 'uploading' } + fileSize.value = file.size ... } const clearUploadState = () => { ... fileName.value = '' + fileSize.value = 0 } return { ... fileName, + fileSize, uploadAndForward, clearUploadState, } }In
FileUpload.vue:-const { uploadingFile, isUploading, uploadAndForward, clearUploadState } = useFileUpload() +const { uploadingFile, isUploading, fileSize, uploadAndForward, clearUploadState } = useFileUpload() <FileCard v-if="uploadingFile" :name="uploadingFile.name" - :size="0" + :size="fileSize" :status="uploadingFile.status" :error="uploadingFile.error" `@remove`="clearUploadState" - `@download`="() => {}" + `@download`="() => {}" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/components/FileUpload.vue` around lines 24 - 36, The FileCard currently receives a hardcoded :size="0" because useFileUpload's uploadingFile doesn't store the original File.size; update the composable (useFileUpload, specifically the uploadAndForward function and the reactive uploadingFile state) to include fileSize (or size) and set it to file.size when an upload starts, expose that property (e.g., uploadingFile.value.size or a separate fileSize ref), then in FileUpload.vue replace :size="0" with the real prop (e.g., :size="uploadingFile.size" or :size="fileSize") so the FileCard shows the correct size and ensure clearUploadState clears the size field as well.backend/app/utils/tool_result_processor.py-302-309 (1)
302-309:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRegister the new weather tool name here too.
This registry special-cases
get_weather, but the MCP server added in this PR exposesget_weather_info. As written, MCP weather results will skip the dedicated formatter and fall back to the generic path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/tool_result_processor.py` around lines 302 - 309, TOOL_PROCESSORS currently maps "get_weather" to _process_weather_result but the new MCP tool uses the name "get_weather_info", so update the registry by adding an entry that maps "get_weather_info" to _process_weather_result alongside the existing "get_weather" key; modify the TOOL_PROCESSORS dict to include "get_weather_info": _process_weather_result so MCP weather results use the dedicated formatter.backend/app/api/v1/endpoints/chat.py-313-315 (1)
313-315:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
temperature/max_tokens/top_pare silently clobbered to defaults when the caller passes0.All five locations use the pattern
if request.temperature is not None: loop_kwargs["temperature"] = request.temperature or 0.7. Theor 0.7short-circuits whenrequest.temperatureis0(a perfectly valid value for greedy/deterministic decoding) or0.0, replacing it with0.7. Same applies fortop_p=0. Formax_tokens=0the fallback to4096is arguably surprising too. Drop theor default— the outeris not Nonecheck is already gating injection.Ruff also flags these lines for E701 (multiple statements on one line); applying the fix below also resolves that.
🛡️ Proposed fix (apply at each of the five locations)
- if request.temperature is not None: loop_kwargs["temperature"] = request.temperature or 0.7 - if request.max_tokens is not None: loop_kwargs["max_tokens"] = request.max_tokens or 4096 - if request.top_p is not None: loop_kwargs["top_p"] = request.top_p or 0.9 + if request.temperature is not None: + loop_kwargs["temperature"] = request.temperature + if request.max_tokens is not None: + loop_kwargs["max_tokens"] = request.max_tokens + if request.top_p is not None: + loop_kwargs["top_p"] = request.top_pAlso applies to: 667-669, 701-703, 774-776, 851-853
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/api/v1/endpoints/chat.py` around lines 313 - 315, The current assignments use "request.temperature or 0.7" (and similar for max_tokens/top_p) which wrongly replaces valid zero values; update each occurrence where loop_kwargs is set from request (e.g., the lines that read if request.temperature is not None: loop_kwargs["temperature"] = request.temperature or 0.7) to assign the raw value guarded by the is not None check (e.g., if request.temperature is not None: loop_kwargs["temperature"] = request.temperature), do the same for request.max_tokens and request.top_p, and expand the single-line if statements into separate lines to avoid E701 (multiple statements on one line); apply this change at all five locations where these patterns appear.frontend/src/renderer/src/stores/chat.ts-374-378 (1)
374-378:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAccessing
m.file_name/m.file_typeon aChatMessagethat doesn't declare them.
ChatMessage(intypes/index.ts) declaresfiles?: ChatFile[]but nofile_name/file_type. The fallback path here readsm.file_name/m.file_typewhich only type-checks if the iterator value is widened toany(which it currently is becauseconv.messagesoriginates from a JSON parse). If strict TS is ever enabled this will fail. Either:
- Add
file_name?: string; file_type?: stringtoChatMessageas legacy compatibility fields (and document them), or- Have the backend always emit
files: [...]so the fallback can be removed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/stores/chat.ts` around lines 374 - 378, The code reads m.file_name / m.file_type on items from conv.messages even though the ChatMessage type (types/index.ts) only defines files?: ChatFile[]; fix by making types explicit: add optional legacy fields file_name?: string and file_type?: string to the ChatMessage declaration (and document them as legacy compatibility) so the fallback path type-checks, or remove the fallback and enforce the backend always emits files: ChatFile[] so msg.files is populated only from m.files; update references in the chat store (the block using m.files, file_name, file_type) accordingly.backend/app/api/v1/endpoints/chat.py-78-99 (1)
78-99:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winServer-local
datetime.now()is labelledAsia/Shanghai.
datetime.now()returns the host’s local time. If the backend ever runs in a container/host that isn’t set toAsia/Shanghai, the system prompt confidently states a wrong timezone — and the LLM will compute date-diffs against a wrong "now". Use a timezone-awarenow:🛡️ Proposed fix
-def _inject_system_prompt(messages: list[dict]) -> list[dict]: - from datetime import datetime - now = datetime.now() +def _inject_system_prompt(messages: list[dict]) -> list[dict]: + from datetime import datetime + from zoneinfo import ZoneInfo + now = datetime.now(ZoneInfo("Asia/Shanghai"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/api/v1/endpoints/chat.py` around lines 78 - 99, The system prompt in _inject_system_prompt uses naive datetime.now(), which can be wrong if the host timezone differs; change it to a timezone-aware now using ZoneInfo("Asia/Shanghai") (e.g., import ZoneInfo from zoneinfo and call datetime.now(ZoneInfo("Asia/Shanghai"))) and then use that tz-aware now to build current_date, current_weekday and current_time so the date_prompt reliably states Asia/Shanghai time; keep the rest of the logic (has_system check and prepending/updating the "system" message) unchanged.frontend/src/renderer/src/views/MemoryView.vue-290-305 (1)
290-305:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSilent failures in
saveEdit/handleDeleteFact.Both
catch {}blocks swallow errors without any user-visible feedback or logging. If the backend update/delete fails, the UI quietly desyncs from server state (edit dialog closes or item appears to be deleted while still present after refresh). Surface the error via a toast/notification (or at minimum re-throw / log), so the user knows the action did not succeed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/views/MemoryView.vue` around lines 290 - 305, The empty catch blocks in saveEdit and handleDeleteFact swallow errors and hide failures from users; update these handlers (saveEdit and handleDeleteFact) to surface failures by catching the error and either logging it (e.g., to console or process logger) and showing a user-facing notification/toast, or re-throwing after logging, when memoryStore.updateFact(...) or memoryStore.deleteFact(...) rejects (include agentStore.activeAgent?.id when constructing context); ensure editingFactId/editingContent are only reset after a successful update so the UI does not desync.frontend/src/renderer/src/views/WorkspaceView.vue-551-555 (1)
551-555:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
window.__memoryChatTriggeris set at setup time but never cleared on unmount.The assignment runs once during
<script setup>execution (effectively at mount), butonBeforeUnmountdoes not delete it. After this view is destroyed the global still references the now-unmounted component's closure, leaking the component (and any reactive state it captured viainputText) and silently writing to a staleinputText.valueif another component dispatches a memory trigger. Move the assignment intoonMountedand clear it inonBeforeUnmount.🛠️ Suggested fix
-(window as any).__memoryChatTrigger = handleMemoryChatTriggerDirect - onMounted(async () => { ... + (window as any).__memoryChatTrigger = handleMemoryChatTriggerDirect }) onBeforeUnmount(() => { ... + if ((window as any).__memoryChatTrigger === handleMemoryChatTriggerDirect) { + delete (window as any).__memoryChatTrigger + } })Also applies to: 580-590
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/views/WorkspaceView.vue` around lines 551 - 555, The global window.__memoryChatTrigger is assigned during script setup to the closure handleMemoryChatTriggerDirect which captures reactive inputText, causing a memory leak and stale writes after the component unmounts; move the assignment into onMounted so it only sets window.__memoryChatTrigger when the component is active and then clear (delete or set to undefined) window.__memoryChatTrigger inside onBeforeUnmount to remove the reference to handleMemoryChatTriggerDirect/inputText; update both places where this pattern appears (the handleMemoryChatTriggerDirect assignment and the similar block around lines 580-590) so mounting sets the global and unmounting removes it.
🧹 Nitpick comments (20)
backend/app/utils/web_search_tool.py (1)
99-104: ⚡ Quick winFlatten nested
async withstatements.Both the 360 search and DuckDuckGo functions use nested
async withblocks. Python allows combining these into a single statement for cleaner code.♻️ Proposed simplification
For
_360_search(lines 99-104):try: - async with aiohttp.ClientSession(headers=_360_HEADERS) as session: - async with session.get( - url, - timeout=aiohttp.ClientTimeout(total=15), - allow_redirects=True, - ) as resp: + async with aiohttp.ClientSession(headers=_360_HEADERS) as session, \ + session.get(url, timeout=aiohttp.ClientTimeout(total=15), allow_redirects=True) as resp: if resp.status != 200:For
_ddg_instant_answer(lines 137-142):try: - async with aiohttp.ClientSession() as session: - async with session.get( - _DDG_IA_URL, - params=params, - timeout=aiohttp.ClientTimeout(total=8), - ) as resp: + async with aiohttp.ClientSession() as session, \ + session.get(_DDG_IA_URL, params=params, timeout=aiohttp.ClientTimeout(total=8)) as resp: if resp.status != 200:Also applies to: 137-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/web_search_tool.py` around lines 99 - 104, The nested "async with" blocks in _360_search and _ddg_instant_answer should be flattened into a single combined context manager to simplify the code; replace the pattern "async with aiohttp.ClientSession(...) as session:\n async with session.get(...) as resp:" with a single line combining both contexts (i.e., "async with aiohttp.ClientSession(...) as session, session.get(...) as resp:"), preserving the same timeout, headers, and allow_redirects arguments and keeping all existing error handling and response handling logic inside the combined block.backend/app/utils/tool_parameter_extractor.py (2)
274-277: ⚡ Quick winCombine the conditional branches for cleaner code.
The conditions on lines 274 and 276 both lead to the same outcome. Combine them using a logical
oroperator for better readability.♻️ Proposed simplification
- elif month >= 5 and month <= 8: - enriched = enriched + "下半年" - elif month >= 9: + elif month >= 5: enriched = enriched + "下半年"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/tool_parameter_extractor.py` around lines 274 - 277, The two consecutive branches that both append "下半年" to enriched (the elif month >= 5 and month <= 8 and the elif month >= 9) should be combined into a single condition to simplify logic; locate the block referencing the month variable and enriched and replace the two branches with one (e.g., a single elif that covers month >= 5 or an equivalent simplified condition) so enriched is appended once.
291-291: ⚡ Quick winAdd newline at end of file.
Python files should end with a newline character for POSIX compliance and better tooling compatibility.
♻️ Add trailing newline
return enriched +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/tool_parameter_extractor.py` at line 291, Add a single newline character at the end of the file so it ends with a trailing newline; ensure the file finishes immediately after the final "return enriched" statement in tool_parameter_extractor.py (i.e., insert a newline/line break after that return).frontend/src/main/services/browser/search.ts (1)
76-80: ⚡ Quick winImprove cleanup error handling.
The empty
catchblock silently swallows any cleanup errors. While this might be intentional to prevent exceptions in the finally block, it makes debugging harder.Consider logging the error or at least adding a comment explaining why errors are ignored.
🔍 Proposed improvement
} finally { try { if (!isViewDestroyed(view)) { view.webContents.close() } - } catch {} + } catch (err) { + console.debug('[BrowserSearch] Cleanup error (ignored):', err) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/main/services/browser/search.ts` around lines 76 - 80, The empty catch around the webContents close hides failures; change the catch to surface at least a warning or explain why it's intentionally ignored: replace the empty catch with a call to your logger (or console.warn/console.error) including context and the caught error (e.g., mention view id or type) for the block that calls view.webContents.close, or if swallowing is deliberate add a concise comment explaining why exceptions are safe to ignore; ensure references to isViewDestroyed(view) and view.webContents.close remain and the logged message provides useful debug context.backend/app/utils/weather_tool.py (2)
183-298: 💤 Low valueRemove unnecessary f-string prefix.
Line 298 uses an f-string without any placeholders. This is unnecessary and should be a regular string literal for clarity and to avoid potential confusion.
🔧 Proposed fix
# ---- 兜底:无法解析,按今天处理 ---- return {"date": today.strftime("%Y-%m-%d"), "type": DateType.TODAY, "day_offset": 0, - "error": f"日期格式无法识别,已为你查询今天天气"} + "error": "日期格式无法识别,已为你查询今天天气"}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/weather_tool.py` around lines 183 - 298, The final return in parse_query_date uses an unnecessary f-string for the error message (no placeholders) — update the error literal in the final return of parse_query_date (the fallback branch that returns DateType.TODAY with day_offset 0) to a normal string literal instead of an f-string to remove the redundant f-prefix and improve clarity.
872-894: 💤 Low valueRemove unnecessary f-string prefix.
Line 894 uses an f-string without any placeholders, similar to line 298.
🔧 Proposed fix
return asyncio.run(_weather_tool.get_reply(city, date_str)) except Exception as e: logger.warning(f"[WeatherTool] get_weather_reply 异常: {e}") - return f"天气查询暂时不可用,稍后再试哦~" + return "天气查询暂时不可用,稍后再试哦~"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/weather_tool.py` around lines 872 - 894, In get_weather_reply, remove the unnecessary f-string prefix on the static return message; specifically change the return f"天气查询暂时不可用,稍后再试哦~" to a plain string "天气查询暂时不可用,稍后再试哦~" (leave the logger.warning(f"...{e}") intact since it uses a placeholder). This fixes the redundant f-string usage in _weather_tool.get_reply error handling.backend/app/api/attachment_api.py (1)
157-171: ⚡ Quick winConsider logging when falling back to lossy UTF-8 decoding.
The encoding fallback strategy (UTF-8 → GBK → UTF-8 with
errors='ignore') is reasonable, but the final fallback at line 165 can silently drop characters without notifying the user or logging the event. This could lead to data corruption that's hard to debug.📝 Add logging for encoding fallback
text_extensions = {'txt', 'md', 'csv', 'json', 'xml', 'html', 'css', 'js', 'py', 'java', 'cpp', 'c', 'h', 'go', 'rs', 'ts', 'sql', 'yaml', 'yml'} if ext in text_extensions: try: text = file_bytes.decode('utf-8') except UnicodeDecodeError: try: text = file_bytes.decode('gbk') except UnicodeDecodeError: + logger.warning(f"[UploadForward] 文件编码无法识别,使用UTF-8忽略错误模式: {filename}") text = file_bytes.decode('utf-8', errors='ignore') return { "status": "success", "content": text, "type": "text", "filename": filename, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/api/attachment_api.py` around lines 157 - 171, When decoding text files in the ext/text_extensions branch, add a warning log when you hit the final lossy fallback (file_bytes.decode('utf-8', errors='ignore')) so dropped characters are recorded; include identifying context (filename, ext and a snippet or byte-length) and the fact you fell back from UTF-8→GBK→lossy UTF-8 using the project's logger (or the logging module) so callers can trace encoding issues for functions handling file_bytes and filename in attachment_api.py.backend/app/utils/time_tool.py (1)
96-152: ⚖️ Poor tradeoffLunar calendar data is limited to 2025-2030.
The hardcoded
_LUNAR_MONTH_STARTStable only covers years 2025-2030. Queries for dates outside this range will return{"found": False}from_solar_to_lunar().While the code handles this gracefully with fallback messages (e.g., line 1438-1439), consider:
- Adding a prominent comment warning about the date range limitation
- Planning periodic data updates before 2030
- Or integrating a lunar calendar library if long-term coverage is needed
This is acceptable for a 5-year horizon but requires maintenance planning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/time_tool.py` around lines 96 - 152, The _LUNAR_MONTH_STARTS table only covers 2025–2028 (effectively 2025–2030 in surrounding code) so _solar_to_lunar() will return {"found": False} for dates outside that range; update the file by adding a prominent top-of-table comment next to _LUNAR_MONTH_STARTS stating the exact covered years and maintenance expectations, and either expand the data set (extend tuples through 2030), add a TODO with a scheduled reminder to refresh the table before expiry, or replace/augment this hardcoded table by integrating a lunar calendar library call in _solar_to_lunar() as an alternative long-term solution (reference symbols: _LUNAR_MONTH_STARTS and _solar_to_lunar).frontend/src/renderer/src/types/index.ts (2)
60-67: 💤 Low value
reasoning_contentrequired vsuseApi.tsdefaulting it to''.
ChatStreamChunk.reasoning_contentis declared as requiredstring. That works becauseapiStreamalways defaults it to''when the server omits it, but anyone constructing aChatStreamChunkliteral directly (or any provider returning the field optionally) will trip TypeScript without realizing the empty-string convention. Consider making it optional (reasoning_content?: string) or documenting the empty-string contract here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/types/index.ts` around lines 60 - 67, ChatStreamChunk.reasoning_content is currently a required string but other code (apiStream in useApi.ts) treats it as optional and defaults missing values to '', causing possible TypeScript errors for literal construction or other providers; update the ChatStreamChunk interface to make reasoning_content optional (reasoning_content?: string) or declare it as string | undefined so consumers can omit it, and add a short JSDoc comment indicating the empty-string defaulting behavior by apiStream to keep intent clear (refer to ChatStreamChunk and the apiStream usage in useApi.ts).
119-124: 💤 Low valueAvoid
anyin shared types; useunknown.
Record<string, any>defeats TS in any code that consumesSearchResult.metadata. PreferRecord<string, unknown>so consumers are forced to narrow before reading.- metadata: Record<string, any> + metadata: Record<string, unknown>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/types/index.ts` around lines 119 - 124, The SearchResult interface uses metadata: Record<string, any>, which weakens type safety; change metadata to Record<string, unknown> in the SearchResult definition so consumers must explicitly narrow or assert values before using them, and update any call sites that read metadata (references to SearchResult and its metadata property) to perform proper type checks or type guards where needed.frontend/src/renderer/src/stores/memory.ts (2)
75-76: 💤 Low valueTrailing slash before query string in API paths.
/memory/?agent_id=...and/memory/facts/${factId}?...have a/directly before?. Most routers accept this, but it does generate URLs like/memory/?agent_id=xwhich can break strict matchers or generate duplicate route-table entries server-side. Consider dropping the trailing slash:- const query = agentId ? `?agent_id=${agentId}` : '' - memoryData.value = await apiGet<MemoryData>(`/memory/${query}`) + const query = agentId ? `?agent_id=${encodeURIComponent(agentId)}` : '' + memoryData.value = await apiGet<MemoryData>(`/memory${query}`)Note
encodeURIComponentalso covers any agent IDs containing reserved characters.Also applies to: 86-87, 95-96, 137-138, 152-153, 175-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/stores/memory.ts` around lines 75 - 76, The URL construction in memory.ts builds query strings with a trailing slash before the "?" (e.g., in the memoryData assignment where query is computed and passed to apiGet), causing paths like "/memory/?agent_id=..."; update each occurrence (including the similar patterns at the other reported locations) to concatenate the query without the trailing slash and ensure you encode the agentId/factId with encodeURIComponent before interpolation so apiGet is called with "/memory?agent_id=..." or "/memory/facts/{factId}?..." (no slash before '?') and safely encoded IDs.
72-103: 💤 Low valueErrors are silently swallowed; consider surfacing them.
fetchMemory,fetchSummary, andfetchInjectionContentallcatch { ... }without storing the error anywhere, so consumers (e.g.MemoryView.vue) cannot distinguish “no memory yet” from “backend is down / 500”. A singlelastErrorref (or returning the error) would let the UI render a banner without changing the store’s null/empty fallback behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/stores/memory.ts` around lines 72 - 103, Add an error surface to the memory store by introducing a lastError ref and update fetchMemory, fetchSummary, and fetchInjectionContent to capture the caught error (catch (err)) and set lastError.value = err (clearing it on success), so callers can distinguish backend failures from empty/null results; for fetchInjectionContent also return or rethrow the error after setting lastError so the caller can react immediately if needed.backend/app/utils/tool_executor.py (2)
322-325: ⚡ Quick winBusiness-logic strings hardcoded in a generic summary builder.
The phrases about “报名时间 vs 考试时间” and search-date hints couple
build_tool_summaryto one specific domain. Consider parameterizing these instruction lines via the caller (or moving them into a higher-level prompt assembly step), so this helper stays reusable for non-exam/non-search tool chains.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/tool_executor.py` around lines 322 - 325, The helper build_tool_summary in tool_executor.py currently appends domain-specific Chinese instruction strings (e.g., about “报名时间 vs 考试时间” and search-date hints) which couples this generic summary builder to exam/search logic; refactor by removing or parameterizing those hardcoded lines so callers can supply domain-specific guidance (e.g., accept an optional parameter like extra_instructions or instruction_template in build_tool_summary) or move that logic into the higher-level prompt assembly that calls build_tool_summary; ensure callers that need the exam/search wording pass it in and update call sites accordingly so build_tool_summary remains reusable.
73-77: 💤 Low valueMove
import mathto module top-level.
import mathis invoked inside the hotcalculatebranch on every call. Python caches imports, so this is functionally cheap, but it’s idiomatic to declare module imports at the top of the file — and it makes the allowed namespace easier to audit at a glance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/utils/tool_executor.py` around lines 73 - 77, The local import of math inside the calculate branch should be moved to the module top-level; remove the inline "import math" and add "import math" at file top, then update the code that populates allowed_names (the loop over dir(math) that sets allowed_names[name] = getattr(math, name)) to reference the top-level math module so the calculate/eval path no longer performs imports per-call and the allowed namespace is easier to audit.frontend/src/renderer/src/stores/chat.ts (3)
7-93: ⚖️ Poor tradeoffLarge keyword dictionaries embedded in the chat store hurt maintainability.
The 80+ lines of weighted keyword tables (
SEARCH_POSITIVE_KEYWORDS,SEARCH_KNOWLEDGE_BOUNDARY_COMBOS,PERIODIC_EVENT_PATTERNS, …) are pure data and pure business-domain heuristics; baking them intostores/chat.tsmakes the store hard to read and impossible to A/B without a code change. Consider extracting these into a dedicated module (e.g.src/renderer/src/services/searchIntent.ts) exportingdetectSearchIntentandextractSearchQuery, or — better — moving the heuristic to the backend so it stays consistent with the backend'sclassify_request/intent_gateway logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/stores/chat.ts` around lines 7 - 93, The large hard-coded keyword tables (e.g., SEARCH_POSITIVE_KEYWORDS, SEARCH_KNOWLEDGE_BOUNDARY_COMBOS, SEARCH_ENTITY_KEYWORDS, SEARCH_NEGATIVE_OVERRIDE) in stores/chat.ts should be extracted into a dedicated module and replaced with a simple import; create a new module (suggested name: services/searchIntent.ts) that exports functions detectSearchIntent and extractSearchQuery which encapsulate these datasets and heuristics, move all keyword maps and combo rules into that module, update stores/chat.ts to import and call detectSearchIntent/extractSearchQuery, and consider (as a follow-up) migrating the heuristic to backend intent classification to keep frontend and backend consistent with the backend classify_request/intent_gateway logic.
221-228: 💤 Low valueRedundant branch in 上/下半年 selection.
if month >= 5 && month <= 8andelse if month >= 9both append'下半年'. Collapse into one branch:- if (!hasHalf && ['考试', '报名', '录取', '分数', '招聘'].includes(matchedType)) { - // 高考/中考固定在6月举行,始终搜索上半年 - const firstHalfOnly = /高考|中考/.test(coreQuery) - if (firstHalfOnly) enriched += '上半年' - else if (month >= 5 && month <= 8) enriched += '下半年' - else if (month >= 9) enriched += '下半年' - else enriched += '上半年' - } + if (!hasHalf && ['考试', '报名', '录取', '分数', '招聘'].includes(matchedType)) { + // 高考/中考固定在6月举行,始终搜索上半年 + const firstHalfOnly = /高考|中考/.test(coreQuery) + if (firstHalfOnly) enriched += '上半年' + else if (month >= 5) enriched += '下半年' + else enriched += '上半年' + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/stores/chat.ts` around lines 221 - 228, The branch deciding whether to append '上半年' or '下半年' is redundant — collapse the two checks that append '下半年' into one; update the logic in the block guarded by if (!hasHalf && ['考试', '报名', '录取', '分数', '招聘'].includes(matchedType)) so that after handling firstHalfOnly (from coreQuery) you use a single condition on month (e.g., month >= 5) to append '下半年' and otherwise append '上半年', modifying the code that writes to enriched (variables/functions to locate: hasHalf, matchedType, coreQuery, enriched, month).
586-600: 💤 Low valueRedundant
awaiton the synchronousdetectSearchIntent.
detectSearchIntentis declaredfunction detectSearchIntent(message: string): boolean— not async.awaiton a non-thenable is harmless but misleading and adds a microtask hop on every send. Drop theawait, or make the function async if you plan to call out to a remote intent classifier in the future.- const searchNeeded = await detectSearchIntent(content) + const searchNeeded = detectSearchIntent(content)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/stores/chat.ts` around lines 586 - 600, The code wrongly uses await on the synchronous detectSearchIntent function, causing an unnecessary microtask hop; change the call in the send flow to call detectSearchIntent(content) without await (or alternatively make detectSearchIntent async if you intend it to be asynchronous later), update usages around detectSearchIntent and any related variable searchNeeded accordingly, and run type checks to ensure no Promise<boolean> mismatches with extractSearchQuery and subsequent browserSearch usage.backend/app/api/v1/endpoints/chat.py (1)
734-916: 🏗️ Heavy lift
_STREAM_RESPONSEduplicates the sametool_loop_streamplumbing 2–3 times.Inside
_STREAM_RESPONSE, theTOOL_CALLbranch (lines 768-810) and the fallbackelsebranch (lines 845-885) implement essentially the same logic: picktool_loop_streamiffc_supported && fc_tools, otherwise fall back tollm_adapter.chat_stream. Theeventhandler block foretype in ("content", "reasoning", "done")is copied verbatim. Extract a single async helper that yields SSE chunks from atool_loop_streamiterator (and one that proxieschat_stream) to cut ~80 lines and ensure both branches stay in sync.Same applies to
_NON_STREAM_GENERATE(lines 661-693 vs 695-727).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/api/v1/endpoints/chat.py` around lines 734 - 916, The _STREAM_RESPONSE function duplicates the same tool_loop_stream vs chat_stream logic across the TOOL_CALL and fallback branches; extract a single async helper (e.g., stream_from_tools_or_llm) that accepts messages, provider, model, tools (fc_tools), and loop_kwargs and internally: if fc_tools uses tool_loop_stream(get_all_tools_schema()) to iterate events and yields normalized SSE pieces via _sse/_sse_reasoning (handling etype "content"/"reasoning"/"done"), else proxies llm_adapter.chat_stream yielding the same normalized SSE pieces; replace the duplicated blocks in _STREAM_RESPONSE (and mirror the same extraction for _NON_STREAM_GENERATE) to call this helper so both branches share one implementation and maintain state updates to state["content"]/state["reasoning"] in the single helper.frontend/src/renderer/src/composables/useApi.ts (1)
142-150: 💤 Low valueProvide a fallback for
raw.id.
id: raw.idwill end up asundefined(typed asstring) if the server ever sends a chunk withoutid. Defaulting it to''(consistent with how the other fields are treated) keepsChatStreamChunkhonest.- id: raw.id, + id: raw.id || '',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/composables/useApi.ts` around lines 142 - 150, The chunk construction may set id to undefined; update the ChatStreamChunk creation so id uses a safe fallback (e.g., id: raw.id || '') consistent with other fields; locate the code building chunk (variable chunk from raw parsed from dataStr) and change the id assignment to default to an empty string when raw.id is falsy.frontend/src/renderer/src/views/WorkspaceView.vue (1)
670-693: 💤 Low valueTemplate-ref function never cleans up
reasoningRefsentries.
:ref="el => { if (el && msg.id) reasoningRefs[msg.id] = el }"only writes; when a message is removed (e.g.,clearMessages, conversation switch) the corresponding DOM element is unmounted but its key remains inreasoningRefs, growing unboundedly across long sessions. Either delete the entry whenelisnull(Vue calls the function ref withnullon unmount), or rebuild the map frommessagesinstead of accumulating manually.🛠️ Suggested fix
- :ref="el => { if (el && msg.id) reasoningRefs[msg.id] = el }" + :ref="el => { + if (el && msg.id) reasoningRefs[msg.id] = el as HTMLElement + else if (!el && msg.id) delete reasoningRefs[msg.id] + }"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/renderer/src/views/WorkspaceView.vue` around lines 670 - 693, The template ref function for reasoningRefs is only writing entries and never removing them, causing memory growth; update the ref handler (the arrow used on :ref) to remove the corresponding key when Vue calls it with el === null (e.g., if el is falsy and msg.id exists delete reasoningRefs[msg.id]) or switch to recomputing reasoningRefs from the current messages array whenever messages change (ensure you update anywhere using reasoningRefs accordingly); target the existing :ref arrow that references reasoningRefs[msg.id] to implement the cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ddad04e2-c76d-4fe0-95a0-194f985de405
⛔ Files ignored due to path filters (12)
backend/app/__pycache__/__init__.cpython-313.pycis excluded by!**/*.pycbackend/app/api/__pycache__/__init__.cpython-313.pycis excluded by!**/*.pycbackend/app/api/v1/__pycache__/__init__.cpython-313.pycis excluded by!**/*.pycbackend/app/api/ws/__pycache__/__init__.cpython-313.pycis excluded by!**/*.pycbackend/app/core/__pycache__/__init__.cpython-313.pycis excluded by!**/*.pycbackend/app/infrastructure/__pycache__/__init__.cpython-313.pycis excluded by!**/*.pycbackend/app/runtime/__pycache__/__init__.cpython-313.pycis excluded by!**/*.pycbackend/app/runtime/event_bus/__pycache__/__init__.cpython-313.pycis excluded by!**/*.pycbackend/app/runtime/pipeline/__pycache__/__init__.cpython-313.pycis excluded by!**/*.pycfrontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlimage-1.pngis excluded by!**/*.pngimage.pngis excluded by!**/*.png
📒 Files selected for processing (110)
0backend/app/api/attachment_api.pybackend/app/api/v1/endpoints/avatar.pybackend/app/api/v1/endpoints/chat.pybackend/app/api/v1/endpoints/device.pybackend/app/api/v1/endpoints/iot.pybackend/app/api/v1/endpoints/plugin.pybackend/app/api/v1/endpoints/session.pybackend/app/api/v1/endpoints/user.pybackend/app/core/agent/__init__.pybackend/app/core/agent/tool_loop.pybackend/app/core/app_factory.pybackend/app/core/config.pybackend/app/domains/__init__.pybackend/app/domains/companion/dialogue_manager.pybackend/app/domains/companion/persona.pybackend/app/domains/companion/storyteller.pybackend/app/domains/connect/device_tracker.pybackend/app/domains/connect/seamless_follow.pybackend/app/domains/connect/sync_service.pybackend/app/domains/hwctrl/gpio_controller.pybackend/app/domains/hwctrl/mcu_protocol.pybackend/app/domains/hwctrl/relay_manager.pybackend/app/domains/intent_classifier.pybackend/app/domains/iot/custom_device.pybackend/app/domains/iot/device_hub.pybackend/app/domains/iot/ha_adapter.pybackend/app/domains/iot/scene_automation.pybackend/app/domains/iot/xiaomi_adapter.pybackend/app/domains/knowledge/profile/preference_learner.pybackend/app/domains/knowledge/profile/user_profile.pybackend/app/domains/mcp_tools/client.pybackend/app/domains/mcp_tools/registry.pybackend/app/domains/multimodal/image/generator.pybackend/app/domains/multimodal/vision/image_analyzer.pybackend/app/domains/orchestrator.pybackend/app/domains/router.pybackend/app/domains/social/contact_manager.pybackend/app/domains/social/friend_request.pybackend/app/domains/tool_executor.pybackend/app/infrastructure/storage/file_manager.pybackend/app/mcp/__init__.pybackend/app/mcp/servers/__init__.pybackend/app/mcp/servers/time_server.pybackend/app/mcp/servers/weather_server.pybackend/app/mcp/tests/__init__.pybackend/app/mcp/tests/test_mcp_protocol.pybackend/app/runtime/context.pybackend/app/runtime/context/__init__.pybackend/app/runtime/context/pipeline_context.pybackend/app/runtime/event_bus/__init__.pybackend/app/runtime/event_bus/core.pybackend/app/runtime/event_bus/subscriber.pybackend/app/runtime/pipeline/__init__.pybackend/app/runtime/pipeline/base.pybackend/app/runtime/pipeline/context.pybackend/app/runtime/pipeline/engine.pybackend/app/runtime/pipeline/stages/01_wake_word.pybackend/app/runtime/pipeline/stages/02_auth.pybackend/app/runtime/pipeline/stages/03_rate_limit.pybackend/app/runtime/pipeline/stages/04_session.pybackend/app/runtime/pipeline/stages/05_context_build.pybackend/app/runtime/pipeline/stages/06_preprocess.pybackend/app/runtime/pipeline/stages/07_agent_route.pybackend/app/runtime/pipeline/stages/08_llm_inference.pybackend/app/runtime/pipeline/stages/09_tool_execute.pybackend/app/runtime/pipeline/stages/10_memory_extract.pybackend/app/runtime/pipeline/stages/11_emotion_analysis.pybackend/app/runtime/pipeline/stages/12_response_decorate.pybackend/app/runtime/pipeline/stages/13_multi_dispatch.pybackend/app/runtime/pipeline/stages/14_audit_log.pybackend/app/runtime/pipeline/stages/__init__.pybackend/app/runtime/plugin/skill/registry.pybackend/app/runtime/provider/base.pybackend/app/runtime/provider/llm/adapter.pybackend/app/runtime/provider/llm/providers.pybackend/app/schemas/chat.pybackend/app/schemas/common.pybackend/app/schemas/device.pybackend/app/schemas/plugin.pybackend/app/schemas/user.pybackend/app/utils/intent_gateway.pybackend/app/utils/local_handler.pybackend/app/utils/search_intent.pybackend/app/utils/time_tool.pybackend/app/utils/tool_executor.pybackend/app/utils/tool_lazy_loader.pybackend/app/utils/tool_parameter_extractor.pybackend/app/utils/tool_result_processor.pybackend/app/utils/weather_tool.pybackend/app/utils/web_search_tool.pyfrontend/package.jsonfrontend/src/main/index.tsfrontend/src/main/services/browser/index.tsfrontend/src/main/services/browser/search.tsfrontend/src/preload/index.tsfrontend/src/renderer/src/components/FileCard.vuefrontend/src/renderer/src/components/FilePreview.vuefrontend/src/renderer/src/components/FileUpload.vuefrontend/src/renderer/src/components/TitleBar.vuefrontend/src/renderer/src/composables/useApi.tsfrontend/src/renderer/src/composables/useFileUpload.tsfrontend/src/renderer/src/config/api.tsfrontend/src/renderer/src/stores/chat.tsfrontend/src/renderer/src/stores/memory.tsfrontend/src/renderer/src/types/index.tsfrontend/src/renderer/src/views/AvatarView.vuefrontend/src/renderer/src/views/MemoryView.vuefrontend/src/renderer/src/views/WorkspaceView.vuefrontend/tsconfig.web.tsbuildinfo
💤 Files with no reviewable changes (6)
- backend/app/runtime/context.py
- backend/app/schemas/common.py
- backend/app/runtime/pipeline/base.py
- backend/app/runtime/pipeline/engine.py
- backend/app/runtime/event_bus/core.py
- backend/app/runtime/pipeline/stages/init.py
| def _inject_file_content(messages: list[dict], parsed_content: str, file_type: str = "text") -> list[dict]: | ||
| if not parsed_content or not parsed_content.strip(): | ||
| return messages | ||
|
|
||
| # 根据文件类型判断是否是图片 | ||
| is_image = file_type == "image" or parsed_content.startswith("data:image") | ||
|
|
||
| if is_image: | ||
| # 找到最后一条用户消息,将图片内容附加到该消息 | ||
| for i in range(len(messages) - 1, -1, -1): | ||
| if messages[i]["role"] == "user": | ||
| # 提取文字内容和图片 | ||
| text_content = messages[i]["content"] | ||
| # 移除 [图片附件] 标记后的内容 | ||
| if "[图片附件]" in text_content: | ||
| text_content = text_content.split("[图片附件]")[0].strip() | ||
|
|
||
| # 构建多模态消息格式 | ||
| messages[i]["content"] = [ | ||
| {"type": "text", "text": text_content or "请分析这张图片"}, | ||
| {"type": "image_url", "image_url": {"url": parsed_content}}, | ||
| ] | ||
| return messages | ||
| return messages | ||
|
|
||
| # 普通文本内容 | ||
| context_text = ( | ||
| "[用户上传文件内容] 以下是与当前对话相关的文件内容,请参考这些内容回答用户的问题。" | ||
| "如果用户的问题与文件内容无关,请正常回答用户问题,不需要强行关联文件。\n\n" | ||
| + parsed_content | ||
| ) | ||
| return [{"role": "user", "content": context_text}] + messages |
There was a problem hiding this comment.
Non-image file injection breaks message ordering by prepending before the system prompt.
For the text-file path, return [{"role": "user", "content": context_text}] + messages puts the file context as the very first message in the list. In add_message, _inject_file_content is called after _inject_system_prompt (line 552) and _inject_memory (line 554), so the resulting order becomes:
[user(file ctx), system, ...memory..., user(original)]
That places a user turn before the system message, which violates most providers’ ordering expectations and silently degrades response quality. Append the file context to the latest user message (as is done for images), or insert it after the system prompt — not before it.
🛡️ Proposed fix — attach file content to the last user message
- # 普通文本内容
- context_text = (
- "[用户上传文件内容] 以下是与当前对话相关的文件内容,请参考这些内容回答用户的问题。"
- "如果用户的问题与文件内容无关,请正常回答用户问题,不需要强行关联文件。\n\n"
- + parsed_content
- )
- return [{"role": "user", "content": context_text}] + messages
+ # 普通文本内容:附加到最后一条 user 消息
+ context_text = (
+ "[用户上传文件内容] 以下是与当前对话相关的文件内容,请参考这些内容回答用户的问题。"
+ "如果用户的问题与文件内容无关,请正常回答用户问题,不需要强行关联文件。\n\n"
+ + parsed_content
+ )
+ for i in range(len(messages) - 1, -1, -1):
+ if messages[i]["role"] == "user" and isinstance(messages[i].get("content"), str):
+ messages[i]["content"] = f"{messages[i]['content']}\n\n{context_text}"
+ return messages
+ # 无用户消息时回退:插入到 system 之后
+ insert_at = 1 if messages and messages[0].get("role") == "system" else 0
+ return messages[:insert_at] + [{"role": "user", "content": context_text}] + messages[insert_at:]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/api/v1/endpoints/chat.py` around lines 102 - 133, The current
_inject_file_content prepends text file context before the system prompt causing
invalid message ordering; change it to attach the parsed_content to the latest
user message (like the image branch) or, if no user message exists, insert the
new user-context message immediately after the system prompt inserted by
_inject_system_prompt (and after _inject_memory). Locate _inject_file_content in
the chat flow (called by add_message after _inject_system_prompt and
_inject_memory), find the last messages entry with role "user" and append or
merge context_text into that message's content (preserving existing content
formatting), otherwise find the message with role "system" and insert the new
{"role":"user","content":context_text} right after it.
| def _persist_conv(conv_id: str, conv: dict) -> None: | ||
| conv["updated_at"] = datetime.now(timezone.utc).isoformat() | ||
| conversations_store.set(conv_id, conv) | ||
|
|
||
|
|
||
| def _append_user_msg(conv: dict, content: str, file_content: str | None = None) -> dict: | ||
| entry: dict = {"role": "user", "content": content} | ||
| if file_content: | ||
| entry["file_content"] = file_content | ||
| last = conv["messages"][-1] if conv["messages"] else None | ||
| if not last or last != entry: | ||
| conv["messages"].append(entry) | ||
| return entry | ||
| return last | ||
|
|
||
|
|
||
| def _append_assistant_msg(conv: dict, content: str, reasoning: str | None = None, interrupted: bool = False) -> dict: | ||
| entry: dict = {"role": "assistant", "content": content} | ||
| if reasoning: | ||
| entry["reasoning_content"] = reasoning | ||
| if interrupted: | ||
| entry["interrupted"] = True | ||
| last = conv["messages"][-1] if conv["messages"] else None | ||
| if not last or last.get("content") != content or (reasoning and last.get("reasoning_content") != reasoning): | ||
| conv["messages"].append(entry) | ||
| return entry | ||
| return last | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Dead helper functions _append_user_msg / _append_assistant_msg; PEP 8 violations on _PHASE_* / _NON_STREAM_GENERATE / _STREAM_RESPONSE.
Two issues mixed here:
_append_user_msg(200) and_append_assistant_msg(211) are defined but never called —add_messageuses_phase_1_save_user_msgand_PHASE_3_SAVE_ASSISTANT_MSGinstead. The two pairs implement subtly different dedup logic, which is confusing. Either delete the unused pair, or consolidate to a single helper.- Function names
_PHASE_3_SAVE_ASSISTANT_MSG,_NON_STREAM_GENERATE,_STREAM_RESPONSEuse upper-case — they look like module-level constants (Ruff N802). Rename to lowercase to match the rest of the file (_phase_3_save_assistant_msg,_non_stream_generate,_stream_response).
Also, while you're cleaning these up, line 395 has an f-string with no placeholders (Ruff F541): logger.info(f"[API] POST /chat/completions - Starting stream response") → drop the f.
Also applies to: 600-628
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 196-196: Use datetime.UTC alias
Convert to datetime.UTC alias
(UP017)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/api/v1/endpoints/chat.py` around lines 195 - 222, The file
contains two unused helper functions _append_user_msg and _append_assistant_msg
that duplicate deduplication logic already provided by _phase_1_save_user_msg
and _PHASE_3_SAVE_ASSISTANT_MSG; either remove the unused _append_* functions or
merge their logic into the single helper used by add_message and update all call
sites to that helper (search for _append_user_msg, _append_assistant_msg and
replace or delete accordingly). Also rename the uppercase function names
_PHASE_3_SAVE_ASSISTANT_MSG, _NON_STREAM_GENERATE, and _STREAM_RESPONSE to
snake_case (_phase_3_save_assistant_msg, _non_stream_generate, _stream_response)
and update every call/reference to those identifiers. Finally remove the stray
f-prefix from logger.info calls that have no placeholders (e.g.
logger.info(f"[API] POST /chat/completions - Starting stream response")) and
make the same f-prefix removal in the other occurrence block mentioned (around
the second range). Ensure tests/type checks pass after renames.
| import json | ||
| from loguru import logger | ||
| from app.runtime.provider.llm.adapter import llm_adapter | ||
| from app.utils.tool_executor import execute_tool_by_name | ||
| from app.runtime.plugin.skill.registry import SkillRegistry | ||
|
|
||
|
|
||
| def get_all_tools_schema() -> list[dict]: | ||
| try: | ||
| return SkillRegistry.get_openai_tools() | ||
| except Exception as e: | ||
| logger.warning(f"[ToolLoop] get_all_tools_schema failed: {e}") | ||
| return [] | ||
|
|
||
|
|
||
| async def tool_loop( | ||
| messages: list[dict], | ||
| tools: list[dict], | ||
| provider_name: str, | ||
| model: str, | ||
| max_steps: int = 5, | ||
| **kwargs, | ||
| ) -> dict: | ||
| """Tool Loop 核心循环(非流式) | ||
|
|
||
| 返回: | ||
| {"content": str, "reasoning": str, "tool_steps": int} | ||
| """ | ||
| tool_steps = 0 | ||
| duplicate_counter: dict[str, int] = {} | ||
|
|
||
| for step in range(max_steps): | ||
| raw = await llm_adapter.chat( | ||
| messages=messages, | ||
| tools=tools, | ||
| provider_name=provider_name, | ||
| model=model, | ||
| return_raw=True, | ||
| **kwargs, | ||
| ) | ||
|
|
||
| if not isinstance(raw, dict): | ||
| return {"content": str(raw), "reasoning": "", "tool_steps": tool_steps} | ||
|
|
||
| tool_calls = raw.get("tool_calls", []) | ||
| content = raw.get("content", "") or "" | ||
| reasoning = raw.get("reasoning", "") or "" | ||
|
|
||
| if not tool_calls: | ||
| return {"content": content, "reasoning": reasoning, "tool_steps": tool_steps} | ||
|
|
||
| messages.append({ | ||
| "role": "assistant", | ||
| "content": content or None, | ||
| "tool_calls": tool_calls, | ||
| }) | ||
|
|
||
| for tc in tool_calls: | ||
| tc_id = tc.get("id", f"call_{step}") | ||
| fn = tc.get("function", {}) | ||
| tool_name = fn.get("name", "") | ||
| args_str = fn.get("arguments", "{}") | ||
|
|
||
| logger.info(f"[ToolLoop] Step {step + 1}: calling {tool_name}({args_str[:100]})") | ||
|
|
||
| duplicate_counter[tool_name] = duplicate_counter.get(tool_name, 0) + 1 | ||
| if duplicate_counter[tool_name] >= 3: | ||
| logger.warning(f"[ToolLoop] {tool_name} called {duplicate_counter[tool_name]} times, injecting warning") | ||
| messages.append({ | ||
| "role": "tool", | ||
| "tool_call_id": tc_id, | ||
| "content": f"[警告] 你已经连续调用 {tool_name} {duplicate_counter[tool_name]} 次了。如果信息仍然不足,请直接根据已有信息回答用户。", | ||
| }) | ||
| continue | ||
|
|
||
| try: | ||
| args = json.loads(args_str) if args_str else {} | ||
| except json.JSONDecodeError: | ||
| args = {} | ||
|
|
||
| result = await execute_tool_by_name(tool_name, args) | ||
|
|
||
| if len(result) > 2000: | ||
| result = result[:2000] + "...(结果已截断)" | ||
|
|
||
| logger.info(f"[ToolLoop] {tool_name} → {len(result)} chars") | ||
| tool_steps += 1 | ||
|
|
||
| messages.append({ | ||
| "role": "tool", | ||
| "tool_call_id": tc_id, | ||
| "content": result, | ||
| }) | ||
|
|
||
| messages.append({"role": "user", "content": "请根据已获取的信息总结回答用户的问题,不要再调用工具。"}) | ||
| final = await llm_adapter.chat( | ||
| messages=messages, | ||
| provider_name=provider_name, | ||
| model=model, | ||
| **kwargs, | ||
| ) | ||
| final_content = final.get("content", "") if isinstance(final, dict) else str(final) | ||
| final_reasoning = final.get("reasoning", "") if isinstance(final, dict) else "" | ||
| return {"content": final_content, "reasoning": final_reasoning, "tool_steps": tool_steps} | ||
|
|
||
|
|
||
| async def tool_loop_stream( | ||
| messages: list[dict], | ||
| tools: list[dict], | ||
| provider_name: str, | ||
| model: str, | ||
| max_steps: int = 5, | ||
| **kwargs, | ||
| ): | ||
| """Tool Loop 核心循环(流式) | ||
|
|
||
| 每轮 yield: | ||
| - {"type": "content", "content": str} — LLM 文本输出 | ||
| - {"type": "reasoning", "content": str} — 推理/状态提示 | ||
| - {"type": "done", "content": str, "reasoning": str} — 最终结果 | ||
| """ | ||
| tool_steps = 0 | ||
| duplicate_counter: dict[str, int] = {} | ||
|
|
||
| for step in range(max_steps): | ||
| collected_content = "" | ||
| collected_reasoning = "" | ||
| collected_tool_calls: dict[int, dict] = {} | ||
|
|
||
| async for chunk in llm_adapter.chat_stream( | ||
| messages=messages, | ||
| tools=tools, | ||
| provider_name=provider_name, | ||
| model=model, | ||
| **kwargs, | ||
| ): | ||
| content = chunk.get("content", "") | ||
| reasoning = chunk.get("reasoning", "") | ||
| tc_complete = chunk.get("tool_calls_complete") | ||
|
|
||
| if content: | ||
| collected_content += content | ||
| yield {"type": "content", "content": content} | ||
| if reasoning: | ||
| collected_reasoning += reasoning | ||
|
|
||
| if tc_complete: | ||
| for tc in tc_complete: | ||
| idx = tc.get("index", len(collected_tool_calls)) | ||
| collected_tool_calls[idx] = tc | ||
|
|
||
| if not collected_tool_calls: | ||
| yield { | ||
| "type": "done", | ||
| "content": collected_content, | ||
| "reasoning": collected_reasoning, | ||
| } | ||
| return | ||
|
|
||
| messages.append({ | ||
| "role": "assistant", | ||
| "content": collected_content or None, | ||
| "tool_calls": [ | ||
| { | ||
| "id": v.get("id", f"call_{step}_{k}"), | ||
| "type": "function", | ||
| "function": v.get("function", {}), | ||
| } | ||
| for k, v in collected_tool_calls.items() | ||
| ], | ||
| }) | ||
|
|
||
| for idx in sorted(collected_tool_calls.keys()): | ||
| tc_data = collected_tool_calls[idx] | ||
| fn = tc_data.get("function", {}) | ||
| tool_name = fn.get("name", "") | ||
| args_str = fn.get("arguments", "{}") | ||
| tc_id = tc_data.get("id", f"call_{step}_{idx}") | ||
|
|
||
| logger.info(f"[ToolLoop] Step {step + 1} stream: calling {tool_name}({args_str[:100]})") | ||
|
|
||
| duplicate_counter[tool_name] = duplicate_counter.get(tool_name, 0) + 1 | ||
| if duplicate_counter[tool_name] >= 3: | ||
| messages.append({ | ||
| "role": "tool", | ||
| "tool_call_id": tc_id, | ||
| "content": f"[警告] 你已经连续调用 {tool_name} {duplicate_counter[tool_name]} 次了。如果信息仍然不足,请直接根据已有信息回答用户。", | ||
| }) | ||
| continue | ||
|
|
||
| yield {"type": "reasoning", "content": f"正在查询 {tool_name}..."} | ||
|
|
||
| try: | ||
| args = json.loads(args_str) if args_str else {} | ||
| except json.JSONDecodeError: | ||
| args = {} | ||
|
|
||
| result = await execute_tool_by_name(tool_name, args) | ||
|
|
||
| if len(result) > 2000: | ||
| result = result[:2000] + "...(结果已截断)" | ||
|
|
||
| logger.info(f"[ToolLoop] {tool_name} → {len(result)} chars") | ||
| tool_steps += 1 | ||
|
|
||
| messages.append({ | ||
| "role": "tool", | ||
| "tool_call_id": tc_id, | ||
| "content": result, | ||
| }) | ||
|
|
||
| messages.append({"role": "user", "content": "请根据已获取的信息总结回答用户的问题,不要再调用工具。"}) | ||
| async for chunk in llm_adapter.chat_stream( | ||
| messages=messages, | ||
| tools=None, | ||
| provider_name=provider_name, | ||
| model=model, | ||
| **kwargs, | ||
| ): | ||
| content = chunk.get("content", "") | ||
| rc = chunk.get("reasoning", "") | ||
| if content: | ||
| yield {"type": "content", "content": content} | ||
| if rc: | ||
| collected_reasoning += rc | ||
|
|
||
| yield {"type": "done", "content": "", "reasoning": collected_reasoning} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm every import resolves to the tool_loop submodule (not the package __init__)
rg -nP --type=py '\bfrom\s+app\.core\.agent(\.tool_loop)?\s+import\b' -C1
# And verify no one is importing from app.core.agent directly that we'd break
rg -nP --type=py '\bfrom\s+app\.core\.agent\s+import\b'Repository: LuminousCX/LuomiNest
Length of output: 1579
🏁 Script executed:
# Compare the two files to confirm exact duplication
echo "=== Checking if __init__.py and tool_loop.py are identical ==="
diff -u backend/app/core/agent/tool_loop.py backend/app/core/agent/__init__.py
echo ""
echo "=== Also check file sizes ==="
wc -l backend/app/core/agent/__init__.py backend/app/core/agent/tool_loop.pyRepository: LuminousCX/LuomiNest
Length of output: 1320
🏁 Script executed:
# Search for any imports that use the app.core.agent package (without .tool_loop)
# This would catch cases like: from app.core.agent import tool_loop
echo "=== Searching for direct app.core.agent imports (package-level) ==="
rg -nP --type=py 'from\s+app\.core\.agent\s+import'
echo ""
echo "=== Searching for any wildcard imports ==="
rg -nP --type=py 'from\s+app\.core\.agent\s+import\s+\*'Repository: LuminousCX/LuomiNest
Length of output: 176
This file diverges from backend/app/core/agent/tool_loop.py and contains a critical bug in tool_loop_stream.
While __init__.py and tool_loop.py define the same three functions, they are not identical:
-
Missing yield in
tool_loop_stream:__init__.pyis missing the lineyield {"type": "reasoning", "content": reasoning}(present intool_loop.pyaround line 146). This breaks the documented streaming contract—reasoning chunks will not be emitted during iteration, even though the docstring promises them. -
Code duplication:
chat.pyimports exclusively fromapp.core.agent.tool_loop(lines 309, 663, 697, 770, 847), so__init__.pyis dead code that will silently drift from the canonical implementation and create confusion for IDE "go to definition" and refactoring tools. -
No re-export fallback: No code uses the package-level re-export pattern, confirming this file serves no purpose.
Fix: Replace __init__.py entirely with a re-export from tool_loop.py:
Replacement for backend/app/core/agent/__init__.py
from app.core.agent.tool_loop import (
get_all_tools_schema,
tool_loop,
tool_loop_stream,
)
__all__ = ["get_all_tools_schema", "tool_loop", "tool_loop_stream"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/core/agent/__init__.py` around lines 1 - 227, The package
__init__ contains a divergent, duplicated implementation where tool_loop_stream
is missing the yield of reasoning chunks (the line yield {"type": "reasoning",
"content": reasoning}) causing the stream contract to break; replace the entire
contents of the package initializer with a re-export of the canonical
implementations (get_all_tools_schema, tool_loop, tool_loop_stream) from the
single source of truth (tool_loop) so callers use the same functions and avoid
duplication and drift.
| duplicate_counter[tool_name] = duplicate_counter.get(tool_name, 0) + 1 | ||
| if duplicate_counter[tool_name] >= 3: | ||
| logger.warning(f"[ToolLoop] {tool_name} called {duplicate_counter[tool_name]} times, injecting warning") | ||
| messages.append({ | ||
| "role": "tool", | ||
| "tool_call_id": tc_id, | ||
| "content": f"[警告] 你已经连续调用 {tool_name} {duplicate_counter[tool_name]} 次了。如果信息仍然不足,请直接根据已有信息回答用户。", | ||
| }) | ||
| continue |
There was a problem hiding this comment.
duplicate_counter is cumulative, not consecutive — and warns once then never resets.
duplicate_counter is incremented every time a tool is called across all steps, so once any tool has been requested 3+ times in the entire loop, every subsequent call to that same tool is replaced by a warning message — even after the model has corrected course and called something else in between. This will derail long, legitimately repetitive tool chains (e.g. multiple web_search queries for different sub-questions). If the intent is to detect tight loops, track consecutive calls instead (reset on any other tool name).
Also, this same logic is duplicated in backend/app/core/agent/__init__.py’s tool_loop — fix in both places (or, better, delete one — see separate comment on the duplicate file).
Also applies to: 171-178
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/core/agent/tool_loop.py` around lines 61 - 69, The
duplicate_counter logic in tool_loop is tracking cumulative calls per tool
rather than consecutive calls; change it to track consecutive repeats by storing
the last_tool_name and a repeat_count (reset repeat_count to 0 or set
last_tool_name to current tool when a different tool is observed) and use that
repeat_count for the >=3 warning logic before appending the warning message
(refer to duplicate_counter, tool_name, tc_id and the messages.append block);
apply the same fix to the other tool_loop implementation (remove one duplicate
implementation if intended) so both tool_loops reset the counter on any
different tool and only warn for consecutive repeats.
| if rc: | ||
| collected_reasoning += rc | ||
|
|
||
| yield {"type": "done", "content": "", "reasoning": collected_reasoning} |
There was a problem hiding this comment.
collected_reasoning leaks across scopes; broken on max_steps == 0 or no-tool-call flow.
After the per-step for loop exits, line 214 mutates collected_reasoning, which was a local variable defined inside the loop body (line 115). This “works” by accident when at least one iteration ran and that iteration produced tool calls. But:
- If
max_steps=0,collected_reasoningis undefined here →NameError. - The early-return path (line 158) already yields
done, so we never reach line 216 from that path — fine. - The post-loop final pass also drops
contentchunks back intoyield {"type": "content", ...}but never appends them into any accumulator, so the finaldoneevent at line 216 contains an emptycontentfield while the consumer inchat.pyexpects to also receive the final content via the same channel. Thedoneevent’scontentis thus inconsistent with whattool_loopreturns.
Initialize the final-pass accumulators explicitly outside the loop, and make sure the done payload reflects the full final answer.
🛡️ Proposed fix
- messages.append({"role": "user", "content": "请根据已获取的信息总结回答用户的问题,不要再调用工具。"})
- async for chunk in llm_adapter.chat_stream(
- messages=messages,
- tools=None,
- provider_name=provider_name,
- model=model,
- **kwargs,
- ):
- content = chunk.get("content", "")
- rc = chunk.get("reasoning", "")
- if content:
- yield {"type": "content", "content": content}
- if rc:
- collected_reasoning += rc
-
- yield {"type": "done", "content": "", "reasoning": collected_reasoning}
+ messages.append({"role": "user", "content": "请根据已获取的信息总结回答用户的问题,不要再调用工具。"})
+ final_content = ""
+ final_reasoning = ""
+ async for chunk in llm_adapter.chat_stream(
+ messages=messages,
+ tools=None,
+ provider_name=provider_name,
+ model=model,
+ **kwargs,
+ ):
+ content = chunk.get("content", "")
+ rc = chunk.get("reasoning", "")
+ if content:
+ final_content += content
+ yield {"type": "content", "content": content}
+ if rc:
+ final_reasoning += rc
+
+ yield {"type": "done", "content": final_content, "reasoning": final_reasoning}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/core/agent/tool_loop.py` around lines 213 - 216, The bug is that
collected_reasoning and the final content accumulator are only created inside
the per-step loop and thus can be undefined (e.g., max_steps==0) and the final
done event uses an empty content; fix tool_loop by declaring and initializing
accumulators (e.g., collected_reasoning = "" and collected_final_content = "" or
lists) before entering the for/step loop, update the per-step logic that
currently mutates the loop-local collected_reasoning to append into these outer
accumulators, ensure any yielded "content" events also append to
collected_final_content, and change the final yield {"type":"done", "content":
"", "reasoning": collected_reasoning} to use the accumulated content and
reasoning variables so done always returns the full final answer and reasoning
even when zero steps run.
| # 3. 如果同时有时间工具和搜索工具,先执行时间工具获取当前日期 | ||
| # 用于优化搜索查询词(如5月问软考→搜索"下半年") | ||
| current_date_info: str | None = None | ||
| if "get_current_time" in tool_names and "web_search" in tool_names: | ||
| try: | ||
| time_args = _extractor.extract("get_current_time", user_query) | ||
| time_result = _time_tool_instance.get_reply( | ||
| query_type="date", | ||
| user_message=user_query, | ||
| ) | ||
| if time_result: | ||
| current_date_info = time_result | ||
| logger.info(f"[ToolExecutor] 跨工具联动: 获取当前日期 → {current_date_info[:50]}") | ||
| except Exception as e: | ||
| logger.debug(f"[ToolExecutor] 跨工具联动时间获取失败: {e}") | ||
|
|
There was a problem hiding this comment.
Dead code: current_date_info and time_args are computed but never used.
The cross-tool-linkage block builds current_date_info (and extracts time_args) but neither is consumed downstream — execute_single_tool is called for web_search without the date context. Either pass current_date_info into the search query enrichment, or remove the block to avoid wasted work and misleading log output.
♻️ Suggested fix — wire the date context into the search step (or delete the block)
- current_date_info: str | None = None
- if "get_current_time" in tool_names and "web_search" in tool_names:
- try:
- time_args = _extractor.extract("get_current_time", user_query)
- time_result = _time_tool_instance.get_reply(
- query_type="date",
- user_message=user_query,
- )
- if time_result:
- current_date_info = time_result
- logger.info(f"[ToolExecutor] 跨工具联动: 获取当前日期 → {current_date_info[:50]}")
- except Exception as e:
- logger.debug(f"[ToolExecutor] 跨工具联动时间获取失败: {e}")
+ current_date_info: str | None = None
+ if "get_current_time" in tool_names and "web_search" in tool_names:
+ try:
+ current_date_info = _time_tool_instance.get_reply(
+ query_type="date",
+ user_message=user_query,
+ ) or None
+ if current_date_info:
+ logger.info(f"[ToolExecutor] 跨工具联动: 获取当前日期 → {current_date_info[:50]}")
+ except Exception as e:
+ logger.debug(f"[ToolExecutor] 跨工具联动时间获取失败: {e}")Then thread current_date_info into execute_single_tool/web_search to actually influence the query.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/utils/tool_executor.py` around lines 148 - 163, The code computes
current_date_info (and time_args) via _extractor.extract and
_time_tool_instance.get_reply but never uses them; either remove this cross-tool
block or thread the date context into the web search invocation so it actually
influences the query. Specifically, update the place where execute_single_tool
is called for the "web_search" tool to accept an extra parameter (or include in
its args) the current_date_info when present, or delete the entire block that
sets current_date_info/time_args if you don't plan to enrich searches;
references to current_date_info, time_args, _time_tool_instance.get_reply,
_extractor.extract and execute_single_tool/web_search should guide locating and
modifying the code.
| # ---------- 通用路径:走 SkillExecutor ---------- | ||
| raw = await executor.execute(tool_name, args, agent_id=agent_id) | ||
| processed = process_tool_result(tool_name, raw) | ||
| logger.info(f"[ToolExecutor] {tool_name} → 原始 {len(raw)} 字符 → 精简 {len(processed)} 字符") | ||
| return { |
There was a problem hiding this comment.
len(raw) will raise TypeError if executor returns a non-sequence.
executor.execute(...) can return None, a dict, or other non-Sized types. len(raw) (line 271) blows up before process_tool_result ever runs, and the exception falls through to the generic except returning a success: False for what was actually a successful call. Either coerce to str first, or only log the length on the processed value.
🛡️ Proposed fix
- raw = await executor.execute(tool_name, args, agent_id=agent_id)
- processed = process_tool_result(tool_name, raw)
- logger.info(f"[ToolExecutor] {tool_name} → 原始 {len(raw)} 字符 → 精简 {len(processed)} 字符")
+ raw = await executor.execute(tool_name, args, agent_id=agent_id)
+ processed = process_tool_result(tool_name, raw)
+ raw_len = len(raw) if hasattr(raw, "__len__") else len(str(raw)) if raw is not None else 0
+ logger.info(f"[ToolExecutor] {tool_name} → 原始 {raw_len} 字符 → 精简 {len(processed)} 字符")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # ---------- 通用路径:走 SkillExecutor ---------- | |
| raw = await executor.execute(tool_name, args, agent_id=agent_id) | |
| processed = process_tool_result(tool_name, raw) | |
| logger.info(f"[ToolExecutor] {tool_name} → 原始 {len(raw)} 字符 → 精简 {len(processed)} 字符") | |
| return { | |
| # ---------- 通用路径:走 SkillExecutor ---------- | |
| raw = await executor.execute(tool_name, args, agent_id=agent_id) | |
| processed = process_tool_result(tool_name, raw) | |
| raw_len = len(raw) if hasattr(raw, "__len__") else len(str(raw)) if raw is not None else 0 | |
| logger.info(f"[ToolExecutor] {tool_name} → 原始 {raw_len} 字符 → 精简 {len(processed)} 字符") | |
| return { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/utils/tool_executor.py` around lines 268 - 272, The log currently
calls len(raw) which will TypeError if executor.execute returns a non-sized type
(None, dict, etc.); change the logging to avoid calling len on raw directly by
coercing raw to a string or taking len(processed) only. Update the block around
executor.execute and process_tool_result (variables raw, processed and
logger.info) to compute raw_str = str(raw) (or similar) and log len(raw_str) or
simply log only len(processed) so process_tool_result runs without being
preempted by a TypeError and the logger call is always safe.
| const uploadAndForward = async (file: File): Promise<string> => { | ||
| if (currentUploadController) { | ||
| currentUploadController.abort() | ||
| } | ||
| currentUploadController = new AbortController() | ||
|
|
||
| uploadingFile.value = { name: file.name, status: 'uploading' } | ||
| isUploading.value = true | ||
| parsedContent.value = '' | ||
| fileType.value = '' | ||
|
|
||
| try { | ||
| const formData = new FormData() | ||
| formData.append('file', file) | ||
|
|
||
| const resp = await fetch(API_ENDPOINTS.UPLOAD_FORWARD, { | ||
| method: 'POST', | ||
| body: formData, | ||
| signal: currentUploadController.signal, | ||
| }) | ||
|
|
||
| const data = await resp.json() | ||
|
|
||
| if (!resp.ok || data.status === 'error') { | ||
| uploadingFile.value = { name: file.name, status: 'failed', error: data.message || '上传失败' } | ||
| isUploading.value = false | ||
| currentUploadController = null | ||
| return '' | ||
| } | ||
|
|
||
| const content = data.content || '' | ||
| parsedContent.value = content | ||
| fileType.value = data.type || 'text' | ||
| fileName.value = file.name | ||
| uploadingFile.value = { name: file.name, status: 'success', type: data.type, result: content } | ||
| isUploading.value = false | ||
| currentUploadController = null | ||
| return content | ||
| } catch (e: any) { | ||
| if (e.name === 'AbortError') { | ||
| return '' | ||
| } | ||
| uploadingFile.value = { name: file.name, status: 'failed', error: '网络错误,请检查后端服务' } | ||
| isUploading.value = false | ||
| currentUploadController = null | ||
| return '' | ||
| } | ||
| } |
There was a problem hiding this comment.
Add client-side file size and type validation before upload.
The uploadAndForward function uploads files without checking size or type, which can lead to poor user experience and potential backend issues.
Consider adding:
- File size validation: Check
file.sizeagainst a reasonable limit (e.g., 10MB for documents, 50MB for images) before initiating upload - File type validation: Verify
file.typeor extension matches expected formats, even though the UI<input>has anacceptattribute (users can bypass this)
Without these checks:
- Users may unknowingly upload very large files, leading to long waits and potential timeouts
- Unexpected file types may reach the backend, causing parsing errors
- Network bandwidth may be wasted on invalid uploads
🛡️ Proposed validation
const uploadAndForward = async (file: File): Promise<string> => {
+ // Validate file size (10MB limit for documents)
+ const MAX_FILE_SIZE = 10 * 1024 * 1024
+ if (file.size > MAX_FILE_SIZE) {
+ uploadingFile.value = {
+ name: file.name,
+ status: 'failed',
+ error: `文件过大,请上传小于 ${MAX_FILE_SIZE / 1024 / 1024}MB 的文件`
+ }
+ isUploading.value = false
+ return ''
+ }
+
+ // Validate file type
+ const allowedExtensions = [
+ '.jpg', '.jpeg', '.png', '.gif', '.bmp', '.webp',
+ '.pdf', '.docx', '.doc', '.txt', '.md', '.csv',
+ '.json', '.xml', '.html', '.css', '.js', '.py',
+ '.java', '.cpp', '.c', '.h', '.go', '.rs', '.ts',
+ '.sql', '.yaml', '.yml'
+ ]
+ const fileExtension = '.' + file.name.split('.').pop()?.toLowerCase()
+ if (!allowedExtensions.includes(fileExtension)) {
+ uploadingFile.value = {
+ name: file.name,
+ status: 'failed',
+ error: '不支持的文件类型'
+ }
+ isUploading.value = false
+ return ''
+ }
+
if (currentUploadController) {
currentUploadController.abort()
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/renderer/src/composables/useFileUpload.ts` around lines 12 - 59,
Add client-side size and type validation at the start of uploadAndForward before
creating FormData: inspect file.size and file.type (or extension) and reject
early if outside allowed ranges (e.g., documents <= 10MB, images <= 50MB) or
MIME not in an allowlist (e.g., text/plain, application/pdf, image/png,
image/jpeg, application/msword,
application/vnd.openxmlformats-officedocument.wordprocessingml.document); when
rejecting, set uploadingFile.value = { name: file.name, status: 'failed', error:
'<friendly message>' }, set isUploading.value = false, clear
parsedContent.value/fileType.value/fileName.value as appropriate, leave
currentUploadController null, and return ''; keep the rest of uploadAndForward,
fetch to API_ENDPOINTS.UPLOAD_FORWARD, and AbortError handling unchanged.
Summary by CodeRabbit
New Features
Enhancements